mystic perch
@calm totem
Man I got a question but its a quick question no reason to create new thread
Do you guys recommend breaking down stuff small as this to make it readable and easy to debug?
public void acquire(AssetId assetId) {
refCounter.put(assetId, refCounter.getOrDefault(assetId, 0) + 1);
}
to
public void acquire(AssetId assetId) {
int refCount = refCounter.getOrDefault(assetId, 0) + 1;
refCounter.put(assetId, refCount);
}
civic groveBOT
<@456226577798135808>
Man I got a question but its a quick question no reason t...
Detected code, here are some useful tools:
<@&987246399047479336> please have a look, thanks.
calm totem
what you think
mystic perch
Separate code in small method (< 7 lines) vs big method ( > 7 lines), and Adding line for parameters
Putting method parameter on a new line make it easier to read.
Not only easier, but the order of thing is more clear
public void acquire(AssetId assetId) {
int refCount = refCounter.getOrDefault(assetId, 0);
refCount += 1;
refCounter.put(assetId, refCount);
}```this is even better
calm totem
damn so you recommend i make it as much as readable as possible?
example
public AssetMetadata getAssetMetadata(AssetId assetId) {
AssetMetadata assetMetadata = assetDatabase.getAssetMetadata(assetId);
if (assetMetadata != null) {
return assetMetadata;
}
assetMetadata = assetPersistence.getAssetMetadata(assetId);
if (assetMetadata != null) {
assetDatabase.addAssetMetadata(assetMetadata);
}
return assetMetadata;
}
into
public AssetMetadata getAssetMetadata(AssetId assetId) {
AssetMetadata cachedMetadata = assetDatabase.getAssetMetadata(assetId);
if (cachedMetadata != null) {
return cachedMetadata;
}
AssetMetadata persistedMetadata = assetPersistence.getAssetMetadata(assetId);
if (persistedMetadata != null) {
assetDatabase.addAssetMetadata(persistedMetadata);
}
return persistedMetadata;
}
That method do 2 thing
that why it is hard to read
It get AssetMetadata and put it in database.
make it 2 method
calm totem
Yea I have an AssetDatabase which is Memory-Only cache
and an AssetPersistence which is file IO, reads stuff from file
that method up there is in another class called AssetRepository which relies on AssetDatabase and AssetPersistence
mystic perch
dont name it database if there is no database...
call it MemoryAsset
The other FileAsset
assetPersistence.getAssetMetadata(assetId); vs fileAssets.getAsset(assetId).getMetadata();
Is that not more clear?
then memoryAssets.getAsset(assetId).getMetadata();
I design my code a lot from the interface that I want first.
calm totem
that makes sense generally
but I learned that all these other game engines also called it Database
AssetDatabase, like an In-Memory asset metadata cache
that's why I named it AssetDatabase
mystic perch
Yeah, but if I had to use these libs, I would extend and rename them.
Readability, is very important to update your code long term.
calm totem
you do have a point
i think it would make more sense to declare AssetDatabase as an interface
and them class MemoryAssetDatabase
mystic perch
Database is just the wrong name, open the dictionary and read the word. https://dictionary.cambridge.org/dictionary/english/database
It is not clear
mystic perch
i think it would make more sense to declare AssetDatabase as an interface
But yes, interface would be it.
calm totem
👍 ill see thanks
btw do you recommend new lines in between get and if logic?
like this?
AssetMetadata metadata = assetDatabase.getAssetMetadata(id);
if (metadata == null) {
}
👍 ill check it out
if that was another logic, would you put a new line in between them?
calm totem
just it being a new line already make it better.
yea maybe in a large function
that aside, do you use or follow any "java clean code" resource on internet?
something that I can use as reference
mystic perch
No, i only separate code block that can be separated.
like this ```java
public AssetMetadata getAssetMetadata(AssetId assetId) {
{
AssetMetadata cachedMetadata = assetDatabase.getAssetMetadata(assetId);
if (cachedMetadata != null) {
return cachedMetadata;
}
}
{
AssetMetadata persistedMetadata = assetPersistence.getAssetMetadata(assetId);
if (persistedMetadata != null) {
assetDatabase.addAssetMetadata(persistedMetadata);
}
return persistedMetadata;
}
}``` if you can do this, add a space between.
and after if/else/while/for/etc.
calm totem
👍 thanks
mystic perch
But yes, you can find a style online and make your ide auto format all your code.
you add a .editorconfig file
and the ide format the code as defined.
calm totem
👍 ill check it out thanks
calm totem
yea im working on refactoring my engine
my initial plan was to get an engine running and only focus on design/architecture/logic/how should things go in general
so I wrote codes in certain way to write it as fast as possible
fortunately the way things are designed, i can just easily fix it without breaking the engine, thats kinda the neat part
i just ran into this code, does it look readable to yall
this.assetPath = definition.getScriptPath().toString().replace(ProjectContext.getInstance().getAssetDirectory().toString(), "").substring(1).replace("\\", "/");
i gotta spend a couple days refactoring and applying this clean code bullshit
hella boring
in my opinion, design and architecture is more important than "clean code", because if design and architecture is good, you can always easily "clean your code"
that was my idea when I was working on the engine
i wanted to get a feature working and running as soon as possible and postpone "cleaning the code part" for later
what yall think of that
mystic perch
It could be, just put new . on a new line.
calm totem
Man my brain aint working right at mid night
how do you rewrite to "Readable Code"?
this.assetPath = definition.getScriptPath().toString().replace(ProjectContext.getInstance().getAssetDirectory().toString(), "").substring(1).replace("\\", "/");
What I came up with
String assetDirectoryPath = ProjectContext.getInstance().getAssetDirectory().toString();
String scriptPath = definition.getScriptPath().toString();
String scriptPathRelative = scriptPath.replace(assetDirectoryPath, "");
String scriptPathRelativeRemoveInitialSlash = scriptPathRelative.substring(1);
String scriptPathRelativeConvertToLinuxStylePath = scriptPathRelativeRemoveInitialSlash.replace("\\", "/");
this.assetPath = scriptPathRelativeConvertToLinuxStylePath;
hold on I got a better idea bro, i take that back, my brain working really good
just add a comment
// Take the Absolute Path, Turn it into Relative Path, Relative to Project's Asset Directory, Then Remove Initial Slash, Then Replace the window's style Path into linux style path
this.assetPath = definition.getScriptPath().toString().replace(ProjectContext.getInstance().getAssetDirectory().toString(), "").substring(1).replace("\\", "/");
that should take care of it
@mystic perch what you think
mystic perch
No, I can't read it. Just put each method call on a new line
calm totem
the user should read the comment part instead
mystic perch
It is neatly the same.
I do not trust comment.
I've had time when the comment said one thing the code did another
calm totem
that makes sense
mystic perch
A 3 days bug...
Because I trusted someone else.
A 1200$ bug to put it in perspective.
calm totem
yea that right, not only that, debugging it might be hard
mystic perch
var assetDir = ProjectContext.getInstance()
.getAssetDirectory()
.toString();
this.assetPath = definition.getScriptPath()
.toString()
.replace(assetDir, "")
.substring(1)
.replace("\\", "/");
Is that not better?
calm totem
yeah that makes sense
mystic perch
When reading it, you can execute it in order line by line
calm totem
👍 thanks
I merged it with the master branch even though things are not really ready yet
ill finish the refactoring the rest of the code later
but that's what I ended up with
civic groveBOT
@mystic perch