Feedback from developers regarding GitHub

General Web Dev
1

I am working on my GitHub and trying to upload projects that I can showcase for a potential internship. I would appreciate it if you could take a look and share your thoughts and feedback. Is it advanced enough for a student to showcase? See below for the link to the repos:

https://github.com/Kerem1989/JavaEE-Webshop

2

Yes I think is advanced enough.
I only have a look at the Springboot project.
I haven’t done Springboot for a long time now but I’ll try to help.
We are going to have a back and for, so that way the review is split in pieces.

Now let’s pretend this project will be extremely successful and it will grow in the future. Remember the employer you want to showcase your project will look a staff like this.

So let’s start with weatherpage.html.

I know the css is small, but let’s do good practice an move it to a separate file.

I notice that this repeat over and over (see below) the same code. Just because the image is different. Imaging you add more weather stations more repeats. If you need to make a change in your model you have to make a lot of changes here. Should be only one place

<div class="weather-box">
    <img th:src="@{/images/MET.jpg}" width="250" height="74">

    <div class="weather-data">
        <div class="weather-data-title">Valid Time:</div>
        <div th:text="${displayWarmestWeather.validTime}"></div>
    </div>
    <div class="weather-data">
        <div class="weather-data-title">Temperature:</div>
        <div th:text="${displayWarmestWeather.temperature}"></div>
    </div>
    <div class="weather-data">
        <div class="weather-data-title">Humidity:</div>
        <div th:text="${displayWarmestWeather.humidity}"></div>
    </div>
    <div class="weather-data">
        <div class="weather-data-title">Provider:</div>
        <div th:text="${displayWarmestWeather.provider}"></div>
    </div>
</div>

Change your model to add the image string and use a technique like this:

How to put variable to img src path in Spring with thymeleaf?

Do the refactoring and we’ll continue with the rest of the review

1 Like
3

I do not understand. Your title is Feedback from developers regarding GitHub yet it is in the General Web Dev category and is tagged Java. Are you asking about Git, GitHub, web development, Java or something else? Are you asking about the Java project in GitHub?

1 Like
5

Sorry for the vague request, but i am asking for a Java project on GitHub for a potential employer to see regarding a internship.

6

Thank you very much, i am studying a two year vocational Java educational program, we have not touched html properly yet.

7

Ok. Round two.

    @GetMapping("bestweather")
    public String displayWeather(Model model) {
        Object displayWarmestWeather = tempComparisionService.displayBestWeather();
        model.addAttribute("displayWarmestWeather", displayWarmestWeather);
        return "weatherpage";
    }

Never use Object as type. Be specific.

In TempComparisionService.java,

public double calculateBestWeather()

public double findMax(double d1, double d2, double d3)

Should be private, not public. When programmer reading your code see that a method is public is safe to assume that is being used outside the class. Which is not the case here.

public double findMax(double d1, double d2, double d3)

If for example the number of stations increases are you going to continue add parameters to this method?. Use some kind of collection where you can add as much stations as you want, then loop comparing temperature to find a max.

Lastly, for your api if your add more stations, you’ll keep creating new endpoints
Why not take advantage of Springboot Path variable?. For example:

    @GetMapping("/localWeather/{station}")
    public String displayLocalWeather(Model model, @PathVariable("station") String station) {

}

Then inside it use if/switch statement to return the required data.

Now your api call will look like this:

[> http://localhost:8080/localWeather/smhi
[> http://localhost:8080/localWeather/meteo
[> http://localhost:8080/localWeather/met
[> http://localhost:8080/localWeather/whatever

You have to deal with whatever, perhaps returning an error page and listing correct values. But to keep simple just return a default value

That is for now. Next iteration is going to get little be more complicated.

1 Like